[SimplfyCFG] Set `MD_prof` for `select` used for certain conditional simplifications #154426

mtrofin · 2025-08-19T21:35:23Z

There’s a pattern where a branch is conditioned on a conjunction or disjunction that ends up being modeled as a select where the first operand is set to true or the second to false. If the branch has known branch weights, they can be copied to the select. This is worth doing in case later the select gets transformed to something else (i.e. if we know the profile, we should propagate it).

Issue #147390

mtrofin · 2025-08-19T21:35:39Z

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

llvmbot · 2025-08-19T21:45:29Z

@llvm/pr-subscribers-llvm-transforms

Author: Mircea Trofin (mtrofin)

Changes

There’s a pattern where a branch is conditioned on a select where the first operand is set to true. The branch has known branch weights, and they can be copied to the select. This is worth doing in case later the select gets transformed to something else (i.e. if we know the profile, we should propagate it).

Issue #147390

Full diff: https://github.com/llvm/llvm-project/pull/154426.diff

3 Files Affected:

(modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+10)
(modified) llvm/test/Transforms/SimplifyCFG/branch-fold.ll (+13-5)
(modified) llvm/test/Transforms/SimplifyCFG/preserve-branchweights.ll (+17-17)

diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 055e8cadaab76..bd25f14ec85d0 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -203,6 +203,8 @@ static cl::opt<unsigned> MaxJumpThreadingLiveBlocks(
     cl::desc("Limit number of blocks a define in a threaded block is allowed "
              "to be live in"));
 
+extern cl::opt<bool> ProfcheckDisableMetadataFixes;
+
 STATISTIC(NumBitMaps, "Number of switch instructions turned into bitmaps");
 STATISTIC(NumLinearMaps,
           "Number of switch instructions turned into linear mapping");
@@ -4772,6 +4774,14 @@ static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
     fitWeights(NewWeights);
 
     setBranchWeights(PBI, NewWeights[0], NewWeights[1], /*IsExpected=*/false);
+    // Cond may be a select instruction with the first operand set to "true".
+    if (!ProfcheckDisableMetadataFixes)
+      if (auto *SI = dyn_cast<SelectInst>(Cond)) {
+        assert(isa<ConstantInt>(SI->getTrueValue()) &&
+               (dyn_cast<ConstantInt>(SI->getTrueValue())->isOne()));
+        setBranchWeights(SI, NewWeights[0], NewWeights[1],
+                         /*IsExpected=*/false);
+      }
   }
 
   // OtherDest may have phi nodes.  If so, add an entry from PBI's
diff --git a/llvm/test/Transforms/SimplifyCFG/branch-fold.ll b/llvm/test/Transforms/SimplifyCFG/branch-fold.ll
index 2f5fb4f33013d..b51398dd6222d 100644
--- a/llvm/test/Transforms/SimplifyCFG/branch-fold.ll
+++ b/llvm/test/Transforms/SimplifyCFG/branch-fold.ll
@@ -1,12 +1,12 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals
 ; RUN: opt < %s -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck %s
 
 define void @test(ptr %P, ptr %Q, i1 %A, i1 %B) {
 ; CHECK-LABEL: @test(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[A_NOT:%.*]] = xor i1 [[A:%.*]], true
-; CHECK-NEXT:    [[BRMERGE:%.*]] = select i1 [[A_NOT]], i1 true, i1 [[B:%.*]]
-; CHECK-NEXT:    br i1 [[BRMERGE]], label [[B:%.*]], label [[COMMON_RET:%.*]]
+; CHECK-NEXT:    [[BRMERGE:%.*]] = select i1 [[A_NOT]], i1 true, i1 [[B:%.*]], !prof [[PROF0:![0-9]+]]
+; CHECK-NEXT:    br i1 [[BRMERGE]], label [[B:%.*]], label [[COMMON_RET:%.*]], !prof [[PROF0]]
 ; CHECK:       common.ret:
 ; CHECK-NEXT:    ret void
 ; CHECK:       b:
@@ -15,9 +15,9 @@ define void @test(ptr %P, ptr %Q, i1 %A, i1 %B) {
 ;
 
 entry:
-  br i1 %A, label %a, label %b
+  br i1 %A, label %a, label %b, !prof !0
 a:
-  br i1 %B, label %b, label %c
+  br i1 %B, label %b, label %c, !prof !1
 b:
   store i32 123, ptr %P
   ret void
@@ -146,3 +146,11 @@ Succ:
 }
 
 declare void @dummy()
+
+!0 = !{!"branch_weights", i32 3, i32 7}
+!1 = !{!"branch_weights", i32 11, i32 4}
+;.
+; CHECK: attributes #[[ATTR0:[0-9]+]] = { nounwind ssp memory(read) uwtable }
+;.
+; CHECK: [[PROF0]] = !{!"branch_weights", i32 138, i32 12}
+;.
diff --git a/llvm/test/Transforms/SimplifyCFG/preserve-branchweights.ll b/llvm/test/Transforms/SimplifyCFG/preserve-branchweights.ll
index 0f78e236b4248..7d1153d5c0a0e 100644
--- a/llvm/test/Transforms/SimplifyCFG/preserve-branchweights.ll
+++ b/llvm/test/Transforms/SimplifyCFG/preserve-branchweights.ll
@@ -268,8 +268,8 @@ define void @test7(i1 %a, i1 %b) {
 ; CHECK-LABEL: @test7(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[C:%.*]] = or i1 [[B:%.*]], false
-; CHECK-NEXT:    [[BRMERGE:%.*]] = select i1 [[A:%.*]], i1 true, i1 [[C]]
-; CHECK-NEXT:    br i1 [[BRMERGE]], label [[Y:%.*]], label [[Z:%.*]], !prof [[PROF6:![0-9]+]]
+; CHECK-NEXT:    [[BRMERGE:%.*]] = select i1 [[A:%.*]], i1 true, i1 [[C]], !prof [[PROF6:![0-9]+]]
+; CHECK-NEXT:    br i1 [[BRMERGE]], label [[Y:%.*]], label [[Z:%.*]], !prof [[PROF6]]
 ; CHECK:       common.ret:
 ; CHECK-NEXT:    ret void
 ; CHECK:       Y:
@@ -557,9 +557,9 @@ return:
 define i32 @SimplifyCondBranchToCondBranch(i1 %cmpa, i1 %cmpb) {
 ; CHECK-LABEL: @SimplifyCondBranchToCondBranch(
 ; CHECK-NEXT:  block1:
-; CHECK-NEXT:    [[BRMERGE:%.*]] = select i1 [[CMPA:%.*]], i1 true, i1 [[CMPB:%.*]]
-; CHECK-NEXT:    [[DOTMUX:%.*]] = select i1 [[CMPA]], i32 0, i32 2, !prof [[PROF13:![0-9]+]]
-; CHECK-NEXT:    [[OUTVAL:%.*]] = select i1 [[BRMERGE]], i32 [[DOTMUX]], i32 1, !prof [[PROF14:![0-9]+]]
+; CHECK-NEXT:    [[BRMERGE:%.*]] = select i1 [[CMPA:%.*]], i1 true, i1 [[CMPB:%.*]], !prof [[PROF13:![0-9]+]]
+; CHECK-NEXT:    [[DOTMUX:%.*]] = select i1 [[CMPA]], i32 0, i32 2, !prof [[PROF14:![0-9]+]]
+; CHECK-NEXT:    [[OUTVAL:%.*]] = select i1 [[BRMERGE]], i32 [[DOTMUX]], i32 1, !prof [[PROF13]]
 ; CHECK-NEXT:    ret i32 [[OUTVAL]]
 ;
 block1:
@@ -584,9 +584,9 @@ define i32 @SimplifyCondBranchToCondBranchSwap(i1 %cmpa, i1 %cmpb) {
 ; CHECK-NEXT:  block1:
 ; CHECK-NEXT:    [[CMPA_NOT:%.*]] = xor i1 [[CMPA:%.*]], true
 ; CHECK-NEXT:    [[CMPB_NOT:%.*]] = xor i1 [[CMPB:%.*]], true
-; CHECK-NEXT:    [[BRMERGE:%.*]] = select i1 [[CMPA_NOT]], i1 true, i1 [[CMPB_NOT]]
-; CHECK-NEXT:    [[DOTMUX:%.*]] = select i1 [[CMPA_NOT]], i32 0, i32 2, !prof [[PROF15:![0-9]+]]
-; CHECK-NEXT:    [[OUTVAL:%.*]] = select i1 [[BRMERGE]], i32 [[DOTMUX]], i32 1, !prof [[PROF16:![0-9]+]]
+; CHECK-NEXT:    [[BRMERGE:%.*]] = select i1 [[CMPA_NOT]], i1 true, i1 [[CMPB_NOT]], !prof [[PROF15:![0-9]+]]
+; CHECK-NEXT:    [[DOTMUX:%.*]] = select i1 [[CMPA_NOT]], i32 0, i32 2, !prof [[PROF16:![0-9]+]]
+; CHECK-NEXT:    [[OUTVAL:%.*]] = select i1 [[BRMERGE]], i32 [[DOTMUX]], i32 1, !prof [[PROF15]]
 ; CHECK-NEXT:    ret i32 [[OUTVAL]]
 ;
 block1:
@@ -609,9 +609,9 @@ define i32 @SimplifyCondBranchToCondBranchSwapMissingWeight(i1 %cmpa, i1 %cmpb)
 ; CHECK-NEXT:  block1:
 ; CHECK-NEXT:    [[CMPA_NOT:%.*]] = xor i1 [[CMPA:%.*]], true
 ; CHECK-NEXT:    [[CMPB_NOT:%.*]] = xor i1 [[CMPB:%.*]], true
-; CHECK-NEXT:    [[BRMERGE:%.*]] = select i1 [[CMPA_NOT]], i1 true, i1 [[CMPB_NOT]]
-; CHECK-NEXT:    [[DOTMUX:%.*]] = select i1 [[CMPA_NOT]], i32 0, i32 2, !prof [[PROF17:![0-9]+]]
-; CHECK-NEXT:    [[OUTVAL:%.*]] = select i1 [[BRMERGE]], i32 [[DOTMUX]], i32 1, !prof [[PROF18:![0-9]+]]
+; CHECK-NEXT:    [[BRMERGE:%.*]] = select i1 [[CMPA_NOT]], i1 true, i1 [[CMPB_NOT]], !prof [[PROF17:![0-9]+]]
+; CHECK-NEXT:    [[DOTMUX:%.*]] = select i1 [[CMPA_NOT]], i32 0, i32 2, !prof [[PROF18:![0-9]+]]
+; CHECK-NEXT:    [[OUTVAL:%.*]] = select i1 [[BRMERGE]], i32 [[DOTMUX]], i32 1, !prof [[PROF17]]
 ; CHECK-NEXT:    ret i32 [[OUTVAL]]
 ;
 block1:
@@ -1114,12 +1114,12 @@ exit:
 ; CHECK: [[PROF10]] = !{!"branch_weights", i32 8, i32 33}
 ; CHECK: [[PROF11]] = !{!"branch_weights", i32 112017436, i32 -735157296}
 ; CHECK: [[PROF12]] = !{!"branch_weights", i32 3, i32 5}
-; CHECK: [[PROF13]] = !{!"branch_weights", i32 22, i32 12}
-; CHECK: [[PROF14]] = !{!"branch_weights", i32 34, i32 21}
-; CHECK: [[PROF15]] = !{!"branch_weights", i32 33, i32 14}
-; CHECK: [[PROF16]] = !{!"branch_weights", i32 47, i32 8}
-; CHECK: [[PROF17]] = !{!"branch_weights", i32 6, i32 2}
-; CHECK: [[PROF18]] = !{!"branch_weights", i32 8, i32 2}
+; CHECK: [[PROF13]] = !{!"branch_weights", i32 34, i32 21}
+; CHECK: [[PROF14]] = !{!"branch_weights", i32 22, i32 12}
+; CHECK: [[PROF15]] = !{!"branch_weights", i32 47, i32 8}
+; CHECK: [[PROF16]] = !{!"branch_weights", i32 33, i32 14}
+; CHECK: [[PROF17]] = !{!"branch_weights", i32 8, i32 2}
+; CHECK: [[PROF18]] = !{!"branch_weights", i32 6, i32 2}
 ; CHECK: [[PROF19]] = !{!"branch_weights", i32 99, i32 1}
 ; CHECK: [[PROF20]] = !{!"branch_weights", i32 1, i32 99}
 ; CHECK: [[PROF21]] = !{!"branch_weights", i32 199, i32 1}

mtrofin · 2025-08-20T00:26:00Z

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

+  if (!ProfcheckDisableMetadataFixes)
+    if (auto *SI = dyn_cast<SelectInst>(PBI->getCondition()))
+      if (!MDWeights.empty()) {
+        assert(isSelectInRoleOfConjunctionOrDisjunction(SI));


Note: I preferred assert-ing here (and below) because the expectation is the condition comes from createLogicalOp and the only reason we get a select is because of how or or and get lowered there.

nikic

I'm not sure on this one. The thing is that the canonical form here is a plain and/or, and the only reason we use select is to prevent poison-propagation issues. Improvements to frontend information (e.g. if you slap noundef on the relevant arguments) or analysis may end up converting the select to and/or, in which case we won't be able to preserve the prof metadata. Cases where better analysis has adverse effects are always awkward.

Do you have an example where duplicating this information on the select ends up being useful? After all, these weights are already on the branch over the select.

mtrofin · 2025-08-26T13:59:05Z

I'm not sure on this one. The thing is that the canonical form here is a plain and/or, and the only reason we use select is to prevent poison-propagation issues. Improvements to frontend information (e.g. if you slap noundef on the relevant arguments) or analysis may end up converting the select to and/or, in which case we won't be able to preserve the prof metadata. Cases where better analysis has adverse effects are always awkward.
Do you have an example where duplicating this information on the select ends up being useful? After all, these weights are already on the branch over the select.

The approach is defensive rather than example-driven, because at large scale, places where profile information starts deteriorating are hard to identify, and it's also hard to know there's a problem - hence the approach of "if an instruction can have MD_profile, and it's known, let's capture it, otherwise mark it as unknown". This is compounded by changes that break downstream assumptions. In this case, the profile info is easily known, so it seems prudent to capture it rather than marking it unknown.

Not sure what you mean by "better analysis has adverse effects"?

david-xl · 2025-08-27T18:04:55Z

llvm/test/Transforms/SimplifyCFG/preserve-branchweights.ll

-; CHECK-NEXT:    [[BRMERGE:%.*]] = select i1 [[CMPA:%.*]], i1 true, i1 [[CMPB:%.*]]
-; CHECK-NEXT:    [[DOTMUX:%.*]] = select i1 [[CMPA]], i32 0, i32 2, !prof [[PROF13:![0-9]+]]
-; CHECK-NEXT:    [[OUTVAL:%.*]] = select i1 [[BRMERGE]], i32 [[DOTMUX]], i32 1, !prof [[PROF14:![0-9]+]]
+; CHECK-NEXT:    [[BRMERGE:%.*]] = select i1 [[CMPA:%.*]], i1 true, i1 [[CMPB:%.*]], !prof [[PROF13:![0-9]+]]


two selects in the same block with the same condition should have identical prof data to be consisitent.

Nice find!

Indeed, I had a bug. While looking into it, I found another one - see the patch stacked below this one.

…simplifications

mtrofin requested a review from david-xl August 19, 2025 21:44

mtrofin marked this pull request as ready for review August 19, 2025 21:44

llvmbot added the llvm:transforms label Aug 19, 2025

mtrofin force-pushed the users/mtrofin/08-19-_simplfycfg_set_md_prof_for_select_used_for_certain_conditional_simplifications branch from af93acd to 37e4c28 Compare August 20, 2025 00:23

mtrofin requested a review from nikic August 20, 2025 00:24

mtrofin commented Aug 20, 2025

View reviewed changes

mtrofin mentioned this pull request Aug 20, 2025

[IR] Add MD_prof to the Keep list of dropUBImplyingAttrsAndMetadata #154635

Open

mtrofin requested review from alanzhao1 and rnk August 21, 2025 00:34

mtrofin mentioned this pull request Aug 21, 2025

[SimplifyCFG] Set branch weights when merging conditional store to address #154841

Open

mtrofin force-pushed the users/mtrofin/08-19-_simplfycfg_set_md_prof_for_select_used_for_certain_conditional_simplifications branch 2 times, most recently from 961489a to 386761d Compare August 25, 2025 20:17

mtrofin requested a review from snehasish August 25, 2025 20:19

mtrofin mentioned this pull request Aug 25, 2025

[NFC] Leave a comment in Local.cpp about debug info & sample profiling #155296

Open

nikic reviewed Aug 26, 2025

View reviewed changes

mtrofin force-pushed the users/mtrofin/08-19-_simplfycfg_set_md_prof_for_select_used_for_certain_conditional_simplifications branch from 386761d to 4fea73e Compare August 27, 2025 17:13

david-xl reviewed Aug 27, 2025

View reviewed changes

[SimplfyCFG] Set MD_prof for select used for certain conditional …

34f5a42

…simplifications

mtrofin changed the base branch from main to users/mtrofin/08-28-_simplifycfg_probabilities_associated_with_same_condition_are_constant August 28, 2025 01:36

mtrofin force-pushed the users/mtrofin/08-19-_simplfycfg_set_md_prof_for_select_used_for_certain_conditional_simplifications branch from 4fea73e to 34f5a42 Compare August 28, 2025 01:36

mtrofin mentioned this pull request Aug 28, 2025

[SimplifyCFG] Probabilities associated with same condition are constant #155734

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SimplfyCFG] Set `MD_prof` for `select` used for certain conditional simplifications #154426

[SimplfyCFG] Set `MD_prof` for `select` used for certain conditional simplifications #154426

mtrofin commented Aug 19, 2025 •

edited

Loading

Uh oh!

mtrofin commented Aug 19, 2025 •

edited

Loading

Uh oh!

llvmbot commented Aug 19, 2025

Uh oh!

mtrofin Aug 20, 2025

Uh oh!

nikic left a comment

Uh oh!

mtrofin commented Aug 26, 2025

Uh oh!

david-xl Aug 27, 2025

Uh oh!

mtrofin Aug 28, 2025

Uh oh!

Uh oh!

[SimplfyCFG] Set MD_prof for select used for certain conditional simplifications #154426

Are you sure you want to change the base?

[SimplfyCFG] Set MD_prof for select used for certain conditional simplifications #154426

Conversation

mtrofin commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrofin commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Aug 19, 2025

Uh oh!

mtrofin Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

mtrofin commented Aug 26, 2025

Uh oh!

david-xl Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

mtrofin Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[SimplfyCFG] Set `MD_prof` for `select` used for certain conditional simplifications #154426

[SimplfyCFG] Set `MD_prof` for `select` used for certain conditional simplifications #154426

mtrofin commented Aug 19, 2025 •

edited

Loading

mtrofin commented Aug 19, 2025 •

edited

Loading